Skip to content

Conversation

@kayarhabby
Copy link

1 - Initialisation de la carte
2 - Gestion des controllers (FullScream, Dessin, Reset, Fichier, Navigation, Scale)
3 - Gestion des couches par groupe
4 - Gestion des couches d'objets et surbrillance

…carte maplibre et des controllers de base (fullscreen, navigation, scale)
…rement impacté la classé, il est pour l'heure impossible de d'avoir les couches d'objets au niveau du controller
…ObjectsLayer, une fois la tuile changée la distance dessinée est cachée
… voir les couches d'objets qui posent problèmes
…her des données venant de source différent
_DEFAULT_MAP_STYLES = {
'detail': {'weight': 5, 'opacity': 1, 'color': 'yellow', 'arrowColor': '#FF5E00', 'arrowSize': 8},
'others': {'opacity': 0.9, 'fillOpacity': 0.7, 'color': 'yellow'},
'detail': {'weight': 5, 'opacity': 1, 'color': 'mediumpurple', 'arrowColor': '#FF5E00', 'arrowSize': 8},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it deliberate to change the color value?
And the opacity? (the line below)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but i forgot to remove it. It was for test purposes.

Comment on lines 55 to 57
const normalizedGeometries = geometries.map(geometry => {
return geometry;
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the utility of this map loop?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to merge all the geometries into a single geometry in order to construct the GeometryCollection object in the FileStore class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any alteration in this Array.map, unless i miss something it seems useless

class MaplibreDrawControlManager {
constructor(map, options = {}) {
this.map = map;
this.options = {...options};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply this.options = options?
Immutability issues?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I had many problems with object references, so I decided to make a copy of each object instead of using references to solve the issue.

controls: {
draw: {
polygon: {
title: 'Draw Polygon',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to translate locales from geomanOptions?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, I haven't thought much about it. I can take a look and run some tests after I finish working on the view form in Geotrek-admin.

map.setZoom(context.mapview.zoom);
return true;
} else {
if (map !== null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Map could be null here, but not above?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s been there since the first version, so I left it as long as the code worked. I can refactor it further if needed

return true;
} else {
if (map !== null) {
map.fitBounds(this.bounds, {padding : 0, maxZoom : 16}); // Adjust the map to fit the predefined bounds.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

padding: 0 is necessary? It's not the default value?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the default value. Not necessary. I will remove it from the code.

Comment on lines +16 to +26
function getURLParameter(name) {
var paramEncoded = (RegExp('[?|&]' + name + '=' + '(.+?)(&|$)').exec(location.search)||[,null])[1],
paramDecoded = decodeURIComponent(paramEncoded);
if (typeof paramDecoded == 'string') {
try {
return JSON.parse(paramDecoded);
}
catch (e) {}
}
return paramDecoded;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can use URLSearchParams api to get search params?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first, I thought about it, but then everything started crashing and I didn’t know why. So I removed the URLSearchParams and reintegrated the old code that was already there and it worked perfectly, so I left it as is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var paramEncoded = new URLSearchParams(location.search).get(name) should be fine, to be tested

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it, but it doesn't seem to work properly with the screenshot controller.

* @private
*/
function generateUniqueId() {
return `${Math.random().toString(36).substring(2, 9)}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return `${Math.random().toString(36).substring(2, 9)}`;
return window.crypto.randomUUID()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I will try it then.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mostly using Chrome for web development. Since window.crypto.randomUUID() wasn't working in my browser

window.crypto.randomUUID();
VM1024:1 Uncaught TypeError: window.crypto.randomUUID is not a function
    at <anonymous>:1:15

I added a fallback. I don't know if it's better or not let me know if I should leave it as is or just replace it with the older one.

function generateUniqueId() {
    if (window.crypto && typeof window.crypto.randomUUID === 'function') {
        return window.crypto.randomUUID();
    } else {
        // fallback
        return generateFallbackUniqueID();
    }
}

function generateFallbackUniqueID() {
    return `${Math.random().toString(36).substring(2, 9)}`;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, this method only works in secure contexts (meaning under https url) so indeed a fallback could be useful for local testing without certificates

return window.crypto.randomUUID?.() || ${Math.random().toString(36).substring(2, 9)}`;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Comment on lines +12 to +18
// Unescape a string that was escaped using django.utils.html.escape.
text = text.replace(/&lt;/g, '<');
text = text.replace(/&gt;/g, '>');
text = text.replace(/&quot;/g, '"');
text = text.replace(/&#39;/g, "'");
text = text.replace(/&amp;/g, '&');
return text;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be tested but something like this is maybe safer

return Object.assign(document.createElement('u'), { innerHTML:  text }).textContent;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven’t modified this file, as it’s unrelated to the map functionality I’m currently working on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants